Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added print count of rules in the scan wizard #1219

Merged
merged 18 commits into from
Nov 28, 2023

Conversation

hitenkoku
Copy link
Collaborator

What Changed

  • Added print count of rules in the scan wizard

I would appreciate it if you could review when you have time.

@hitenkoku hitenkoku linked an issue Nov 18, 2023 that may be closed by this pull request
@hitenkoku hitenkoku self-assigned this Nov 18, 2023
@hitenkoku hitenkoku added the enhancement New feature or request label Nov 18, 2023
Copy link

codecov bot commented Nov 18, 2023

Codecov Report

Attention: 402 lines in your changes are missing coverage. Please review.

Comparison is base (f6bf6b3) 83.48% compared to head (fe72800) 82.31%.

Files Patch % Lines
src/yaml.rs 9.17% 208 Missing ⚠️
src/main.rs 0.00% 190 Missing ⚠️
src/afterfact.rs 25.00% 3 Missing ⚠️
src/options/profile.rs 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1219      +/-   ##
==========================================
- Coverage   83.48%   82.31%   -1.18%     
==========================================
  Files          27       27              
  Lines       24082    24313     +231     
==========================================
- Hits        20106    20013      -93     
- Misses       3976     4300     +324     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hitenkoku
Copy link
Collaborator Author

Evidence

./1206.exe csv-timeline -d ../hayabusa-sample-evtx
...
Total event log files: 584
Total file size: 137.1 MB

Scan wizard:

✔ Which set of detection rules would you like to load? · 2. Core+ (2193 rules) ( status: test, stable | level: medium, high, critical )
✔ Include Emerging Threats rules? (183 rules) · no
✔ Include Threat Hunting rules? (6 rules) · no
✔ Include deprecated rules? (161 rules) · no
✔ Include noisy rules? (0 rules) · no
✔ Include unsupported rules? (40 rules) · no
✔ Include sysmon rules? (997 rules) · no

@fukusuket
Copy link
Collaborator

@hitenkoku
I have a question!

  1. All event and alert rules (4442 rules) ( status: * | level: informational+ )

I thought that the 4442 rule would match the total number of files (./rules/hayabusa and ./rules/sigma folders), but is there any reason why it doesn't match?

スクリーンショット 2023-11-18 232107

@YamatoSecurity
Copy link
Collaborator

I thought that the 4442 rule would match the total number of files (./rules/hayabusa and ./rules/sigma folders), but is there any reason why it doesn't match?

The way we currently count the rules is by unique rule IDs, not number of .yml files. So two separate process_creation rules are still treated as 1 sigma rule because originally there was only 1 sigma rule.

@fukusuket
Copy link
Collaborator

@YamatoSecurity @hitenkoku
Thank you for comment! I understand the specifications of the current counting method.
In that case, I think the expected result is that the number of rules displayed in the console (4442) will be less than the total number of yml files? 🤔

@hitenkoku
Copy link
Collaborator Author

@YamatoSecurity @fukusuket Thanks for your comment.

I checked excluded and noisy status in output count.

I fixed in 691ee18 and f8cf6e6 .

Copy link
Collaborator

@fukusuket fukusuket left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hitenkoku
Thank you so much for quick fix! I confirmed #1219 (comment) case fixed.

✔ Include noisy rules? (0 rules) · no

The number of noisy rules is 0. Is this the expected behavior?
Could you check it out?

./hayabusa csv-timeline -d ../hayabusa-sample-evtx/ -o out.csv -C

Scan wizard:

? Which set of detection rules would you like to load? ›
  1. Core (1421 rules) ( status: test, stable | level: high, critical )
  2. Core+ (2299 rules) ( status: test, stable | level: medium, high, critical )
  3. Core++ (3829 rules) ( status: experimental, test, stable | level: medium, high, critical )
  4. All alert rules (4079 rules) ( status: * | level: low+ )
❯ 5. All event and alert rules (4168 rules) ( status: * | level: informational+ )

Scan wizard:

✔ Which set of detection rules would you like to load? · 5. All event and alert rules (4168 rules) ( status: * | level: informational+ )
✔ Include deprecated rules? (186 rules) · no
✔ Include noisy rules? (0 rules) · no
✔ Include unsupported rules? (45 rules) · no
✔ Include sysmon rules? (2050 rules) · yes

Loading detection rules. Please wait.

Excluded rules: 31
Noisy rules: 12 (Disabled)

Deprecated rules: 186 (7.18%) (Disabled)
Experimental rules: 950 (36.67%)
Stable rules: 198 (7.64%)
Test rules: 1443 (55.69%)
Unsupported rules: 45 (1.74%) (Disabled)

Hayabusa rules: 161
Sigma rules: 2430
Total enabled detection rules: 2591

@hitenkoku
Copy link
Collaborator Author

@fukusuket Thanks for your comment. I checked noisy rule count.
I fixed noisy rule count miss of wizard output in 0d77142.

Could you check it?

@hitenkoku Thank you so much for quick fix! I confirmed #1219 (comment) case fixed.

✔ Include noisy rules? (0 rules) · no

The number of noisy rules is 0. Is this the expected behavior? Could you check it out?

./hayabusa csv-timeline -d ../hayabusa-sample-evtx/ -o out.csv -C

Scan wizard:

? Which set of detection rules would you like to load? ›
  1. Core (1421 rules) ( status: test, stable | level: high, critical )
  2. Core+ (2299 rules) ( status: test, stable | level: medium, high, critical )
  3. Core++ (3829 rules) ( status: experimental, test, stable | level: medium, high, critical )
  4. All alert rules (4079 rules) ( status: * | level: low+ )
❯ 5. All event and alert rules (4168 rules) ( status: * | level: informational+ )

Scan wizard:

✔ Which set of detection rules would you like to load? · 5. All event and alert rules (4168 rules) ( status: * | level: informational+ )
✔ Include deprecated rules? (186 rules) · no
✔ Include noisy rules? (0 rules) · no
✔ Include unsupported rules? (45 rules) · no
✔ Include sysmon rules? (2050 rules) · yes

Loading detection rules. Please wait.

Excluded rules: 31
Noisy rules: 12 (Disabled)

Deprecated rules: 186 (7.18%) (Disabled)
Experimental rules: 950 (36.67%)
Stable rules: 198 (7.64%)
Test rules: 1443 (55.69%)
Unsupported rules: 45 (1.74%) (Disabled)

Hayabusa rules: 161
Sigma rules: 2430
Total enabled detection rules: 2591

@hitenkoku hitenkoku requested a review from fukusuket November 19, 2023 07:46
Copy link
Collaborator

@fukusuket fukusuket left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hitenkoku
Thank you so much for quick fix! I confirmed #1219 (review) case. LGTM!!🚀

@YamatoSecurity
Copy link
Collaborator

@hitenkoku
Sorry to add some more things. Can I ask the following:

  1. When 1-3 (Core, Core+, Core++) is selected, we should not ask the user if they want to include deprecated rules or unsupported rules as they have a different status and cannot be enabled. Can you change it so we only ask the user if they want to enabled depracted and unsupported when 4 or 5 is selected?

  2. The rule count should include noisy rules but ignore excluded rules.

  3. This one may be hard to implement so if it is too complex then please ignore it. If possible, when asking the user if they want to include certain rules, we should create filters based on the first question. For example, if the user chooses 1. Core ( status: test, stable | level: high, critical ), then when we ask if they want to include noisy rules, we should only count the number of rules that are 1. marked as noisy and 2. has a status of test or stable and a level of high or above.

  4. If the number of rules is 0, then we don't need to ask the user anything so should just ignore that question.

  5. The final count of rules should be the number of .yml files instead of unique rule IDs. I'm very sorry for this one as we changed over to unique rule IDs but as we are now counting the rules based on number of files I think we should switch back so it makes more sense to the user.

This part:

Loading detection rules. Please wait.

Excluded rules: 1878
Noisy rules: 12 (Disabled)

Stable rules: 69 (7.83%)
Test rules: 812 (92.17%)

Hayabusa rules: 19
Sigma rules: 862
Total enabled detection rules: 881

@hitenkoku
Copy link
Collaborator Author

@YamatoSecurity Thanks for your comment.

I fixed follow comment in c9ad9a0 .

I will implement to other comment.

  1. The final count of rules should be the number of .yml files instead of unique rule IDs. I'm very sorry for this one as we changed over to unique rule IDs but as we are now counting the rules based on number of files I think we should switch back so it makes more sense to the user.

This part:

Loading detection rules. Please wait.

Excluded rules: 1878
Noisy rules: 12 (Disabled)

Stable rules: 69 (7.83%)
Test rules: 812 (92.17%)

Hayabusa rules: 19
Sigma rules: 862
Total enabled detection rules: 881

@hitenkoku
Copy link
Collaborator Author

hitenkoku commented Nov 23, 2023

I already implemented following comment. Could you check it?

  1. This one may be hard to implement so if it is too complex then please ignore it. If possible, when asking the user if they want to include certain rules, we should create filters based on the first question. For example, if the user chooses 1. Core ( status: test, stable | level: high, critical ), then when we ask if they want to include noisy rules, we should only count the number of rules that are 1. marked as noisy and 2. has a status of test or stable and a level of high or above.

Scan wizard:

✔ Which set of detection rules would you like to load? · 1. Core (1377 rules) ( status: test, stable | level: high, critical )
✔ Include Emerging Threats rules? (175 rules) · no
✔ Include Threat Hunting rules? (0 rules) · no
✔ Include deprecated rules? (105 rules) · no
✔ Include noisy rules? (0 rules) · no
✔ Include unsupported rules? (20 rules) · no
✔ Include sysmon rules? (626 rules) · no

Scan wizard:

✔ Which set of detection rules would you like to load? · 2. Core+ (2193 rules) ( status: test, stable | level: medium, high, critical )
✔ Include Emerging Threats rules? (183 rules) · no
✔ Include Threat Hunting rules? (6 rules) · no
✔ Include deprecated rules? (161 rules) · no
✔ Include noisy rules? (0 rules) · no
✔ Include unsupported rules? (40 rules) · no
✔ Include sysmon rules? (997 rules) · no

@hitenkoku
Copy link
Collaborator Author

@YamatoSecurity I fixed following comment. Could you check it?

  1. When 1-3 (Core, Core+, Core++) is selected, we should not ask the user if they want to include deprecated rules or unsupported rules as they have a different status and cannot be enabled. Can you change it so we only ask the user if they want to enabled depracted and unsupported when 4 or 5 is selected?

I fixed in e65e1d4

  1. The rule count should include noisy rules but ignore excluded rules.

I fixed in a34f3ae

  1. If the number of rules is 0, then we don't need to ask the user anything so should just ignore that question.

I fixed in bdb4b00.

@YamatoSecurity
Copy link
Collaborator

@hitenkoku Thank you so much!

For this:

✔ Which set of detection rules would you like to load? · 1. Core (1423 rules) ( status: test, stable | level: high, critical )
✔ Include Emerging Threats rules? (190 rules) · yes
✔ Include sysmon rules? (642 rules) · yes

Loading detection rules. Please wait.

Excluded rules: 2870
Noisy rules: 12 (Disabled)

Stable rules: 103 (7.75%)
Test rules: 1226 (92.25%)

Hayabusa rules: 19
Sigma rules: 1310
Total enabled detection rules: 1329

If the user chooses 1 and enables both ET rules and sysmon rules, then shouldn't the Total enabled detection rules be the same as Core (1423 rules) ( status: test, stable | level: high, critical ) ?
Why does the rule count decrease from 1423 to 1329?

@hitenkoku
Copy link
Collaborator Author

thanks for your comment.
I will check it.

@hitenkoku Thank you so much!

For this:

✔ Which set of detection rules would you like to load? · 1. Core (1423 rules) ( status: test, stable | level: high, critical )
✔ Include Emerging Threats rules? (190 rules) · yes
✔ Include sysmon rules? (642 rules) · yes

Loading detection rules. Please wait.

Excluded rules: 2870
Noisy rules: 12 (Disabled)

Stable rules: 103 (7.75%)
Test rules: 1226 (92.25%)

Hayabusa rules: 19
Sigma rules: 1310
Total enabled detection rules: 1329

If the user chooses 1 and enables both ET rules and sysmon rules, then shouldn't the Total enabled detection rules be the same as Core (1423 rules) ( status: test, stable | level: high, critical ) ?
Why does the rule count decrease from 1423 to 1329?

@hitenkoku
Copy link
Collaborator Author

@YamatoSecurity Sorry for my late comment.

I checked following problm and fixed in 7aa6ba5.

Could you check it?

For this:

✔ Which set of detection rules would you like to load? · 1. Core (1423 rules) ( status: test, stable | level: high, critical )
✔ Include Emerging Threats rules? (190 rules) · yes
✔ Include sysmon rules? (642 rules) · yes

Loading detection rules. Please wait.

Excluded rules: 2870
Noisy rules: 12 (Disabled)

Stable rules: 103 (7.75%)
Test rules: 1226 (92.25%)

Hayabusa rules: 19
Sigma rules: 1310
Total enabled detection rules: 1329

If the user chooses 1 and enables both ET rules and sysmon rules, then shouldn't the Total enabled detection rules be the same as Core (1423 rules) ( status: test, stable | level: high, critical ) ? Why does the rule count decrease from 1423 to 1329?

@YamatoSecurity
Copy link
Collaborator

@hitenkoku Thank you so much!
1-3 Core rule count is now working as expected.

However, when I choose the following:

✔ Which set of detection rules would you like to load? · 4. All alert rules (4138 rules) ( status: * | level: low+ )
✔ Include deprecated rules? (184 rules) · yes
✔ Include unsupported rules? (45 rules) · yes
✔ Include noisy rules? (1 rules) · yes
✔ Include sysmon rules? (2029 rules) · yes

I get only 4108 rules enabled. Shouldn't it be 4138?

Output:

Loading detection rules. Please wait.

Excluded rules: 133
Noisy rules: 12

Deprecated rules: 184 (4.48%)
Experimental rules: 1551 (37.76%)
Stable rules: 161 (3.92%)
Test rules: 2167 (52.75%)
Unsupported rules: 45 (1.10%)

Hayabusa rules: 84
Sigma rules: 4024
Total enabled detection rules: 4108

Also, a similar thing with 5. All event and alert rules (4241 rules) ( status: * | level: informational+ ):

✔ Which set of detection rules would you like to load? · 5. All event and alert rules (4241 rules) ( status: * | level: informational+ )
✔ Include deprecated rules? (186 rules) · yes
✔ Include unsupported rules? (45 rules) · yes
✔ Include noisy rules? (12 rules) · yes
✔ Include sysmon rules? (2054 rules) · yes

Loading detection rules. Please wait.

Excluded rules: 31
Noisy rules: 12

Deprecated rules: 186 (4.42%)
Experimental rules: 1553 (36.89%)
Stable rules: 246 (5.84%)
Test rules: 2180 (51.78%)
Unsupported rules: 45 (1.07%)

Hayabusa rules: 173
Sigma rules: 4037
Total enabled detection rules: 4210

Total enabled detection rules should be 4241, not 4210, correct?

@hitenkoku
Copy link
Collaborator Author

@YamatoSecurity I'm sorry to bother you again.

I fixed follow problem in fe72800.

I also confirmed 1.-5. wizard count is matched with rule count output.

Could you check it?

However, when I choose the following:

✔ Which set of detection rules would you like to load? · 4. All alert rules (4138 rules) ( status: * | level: low+ )
✔ Include deprecated rules? (184 rules) · yes
✔ Include unsupported rules? (45 rules) · yes
✔ Include noisy rules? (1 rules) · yes
✔ Include sysmon rules? (2029 rules) · yes

I get only 4108 rules enabled. Shouldn't it be 4138?

Output:

Loading detection rules. Please wait.

Excluded rules: 133
Noisy rules: 12

Deprecated rules: 184 (4.48%)
Experimental rules: 1551 (37.76%)
Stable rules: 161 (3.92%)
Test rules: 2167 (52.75%)
Unsupported rules: 45 (1.10%)

Hayabusa rules: 84
Sigma rules: 4024
Total enabled detection rules: 4108

Also, a similar thing with 5. All event and alert rules (4241 rules) ( status: * | level: informational+ ):

✔ Which set of detection rules would you like to load? · 5. All event and alert rules (4241 rules) ( status: * | level: informational+ )
✔ Include deprecated rules? (186 rules) · yes
✔ Include unsupported rules? (45 rules) · yes
✔ Include noisy rules? (12 rules) · yes
✔ Include sysmon rules? (2054 rules) · yes

Loading detection rules. Please wait.

Excluded rules: 31
Noisy rules: 12

Deprecated rules: 186 (4.42%)
Experimental rules: 1553 (36.89%)
Stable rules: 246 (5.84%)
Test rules: 2180 (51.78%)
Unsupported rules: 45 (1.07%)

Hayabusa rules: 173
Sigma rules: 4037
Total enabled detection rules: 4210

Total enabled detection rules should be 4241, not 4210, correct?

Copy link
Collaborator

@YamatoSecurity YamatoSecurity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hitenkoku Thanks so much! All looks good now!

@hitenkoku
Copy link
Collaborator Author

@fukusuket @YamatoSecurity thanks for your review.
I will merge it.

@hitenkoku hitenkoku merged commit c11d2f9 into main Nov 28, 2023
7 checks passed
@hitenkoku hitenkoku deleted the 1206-print-count-of-rules-in-the-scan-wizard branch December 14, 2023 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Print count of rules in the scan wizard
3 participants